Tab viability: skip non-viable targets, retire redirect useLayoutEffect (#334)#352
Conversation
Retires the redirect useLayoutEffect in ValueNodeWrapper. `getNextOrPrevious` gains an optional `isViable` predicate; `useCommon` composes it from `filterState.visiblePaths` (new `useRawFilterState` hook) and the existing `allowEditFilter`. Tab navigation skips filtered-out and non-editable nodes up front instead of opening and bouncing, eliminating the spurious startEdit/cancelEdit observer pairs on dead-end nodes. KeyDisplay's Tab path (key-edit → value-edit) inherits the same viability check, since both source roles share `getNextOrPreviousAtPath` via useCommon. Behaviour changes: - No `previouslyEditedElement` fallback when no viable next exists; the editor cancels cleanly instead. - Live search hiding the actively-edited node leaves the editing record in the store; the input unmounts via the existing `!isVisible` early-return. Clearing the search later resumes the edit. No off-screen-commit footgun because there's no input to commit through. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bundle size impact
|
| Format | Base raw | PR raw | Δ raw | Base gzip | PR gzip | Δ gzip |
|---|---|---|---|---|---|---|
| esm | 56.05 KB | 55.76 KB | 🟢 -299 B (-0.52%) | 20.09 KB | 20.01 KB | 🟢 -82 B (-0.40%) |
| cjs | 57.58 KB | 57.29 KB | 🟢 -301 B (-0.51%) | 20.17 KB | 20.10 KB | 🟢 -76 B (-0.37%) |
Measured from build/index.{cjs,esm}.js. Gzip at level 9.
- `getNextOrPrevious`: drop the optional `?` on `isViable`. The only no- predicate callers were tests; production (`useCommon`) always supplies one. Tests now pass `acceptAll = () => true` for the pure structural walk. The "absence of the predicate is byte-equivalent" case is gone — trivially true under a required predicate. - `buildLeafNodeData` (inline in `keyboard.ts`) was a near-duplicate of `buildNodeData` in `JsonEditor.tsx`. Extracted the canonical builder to `src/utils/buildNodeData.ts` and reuse from both call sites. `rootName` defaults to `''` for callers that never target the root. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Both review points addressed in 81a354e.
Both differences are non-conflicting — the unified Net diff: −82 lines across |
…te (#334 follow-up) (#353) After the redirect useLayoutEffect retirement, both `previouslyEditedElement` and `tabDirection` were write-only — `recordPreviousEdit` / `setTabDirection` had no readers. Audit confirmed nothing in src/ or test/ consumes either. Drops the two state fields from `EditingStateBundle`, the two action methods from `EditingStore`, the unused `TabDirection` import, and the corresponding calls from `ValueNodeWrapper.tabTo` (the viability-aware `getNextOrPreviousAtPath` already handles non-viable Tab targets up front). Public `useEditing` hook shape shrinks accordingly. Net: -22 lines on EditingProvider, -15 lines on ValueNodeWrapper (~37 removed, 0 added that count beyond moved comment text). Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Closes #334.
Summary
getNextOrPreviousgains an optional 5thisViable?: (nodeData) => booleanpredicate. At every candidate leaf, the function synthesises NodeData for the candidate and callsisViable; onfalse, it recurses with the failed candidate as the new starting point. Sort-aware index derivation matchesbuildNodeDatasemantics so customsearchFilter/allowEditcallbacks see the same shape they get during render.useCommoncomposesisViableTargetfrom the precomputedfilterState.visiblePathsSet (newuseRawFilterStatehook) and the existingallowEditFilter. Predicate isuseCallback-stable across stable inputs. Threads it throughgetNextOrPreviousAtPath. KeyDisplay's Tab path (key-edit → value-edit) inherits the viability check for free.useLayoutEffectremoved fromValueNodeWrapper. Tab no longer fires transientstartEdit/cancelEditobserver pairs on dead-end nodes.User-visible improvements
onEditEventstream: a Tab past N non-viable nodes used to firestartEdit(badNode_1) → cancelEdit(badNode_1) → startEdit(badNode_2) → cancelEdit(badNode_2) → … → startEdit(goodNode). Now it's juststartEdit(goodNode).useLayoutEffectpattern in every value node.filterNodecalls (the visibility primitive built in Filtered collection counts + filter rewrite (#113) #351).Behaviour changes worth flagging
previouslyEditedElementfallback. Today's redirect tries the previously-edited node before giving up; now when no viable Tab target exists, the editor cancels cleanly. Aligns with the "decision up front" model the issue argued for.recordPreviousEdit/previouslyEditedElementlikely become dead state; queued for a follow-up cleanup PR once this is verified stable.!isVisibleguard), the input unmounts, the editing record stays in the store. Clearing the search later resumes the edit. No off-screen-commit footgun because there's no input element to commit through.Test plan
pnpm test keyboard— 6 new cases for theisViablepredicate (skip single, skip run, return null when none viable, candidate-NodeData shape, descend-then-skip, no-predicate backward-compat). Total 21 passing.pnpm test JsonEditor— added 2 cases: Tab-past-non-editable fires NO transient observer events, and live search hiding the open node unmounts cleanly without error. Existing'Tab and Shift+Tab skip filtered-out nodes'(line 1864) still passes — same user-visible behaviour via different mechanism. Total 107 passing.pnpm test— 456 tests across 18 suites passing.pnpm lint && pnpm compile && pnpm build— clean.editorRef.startEdit(path, { force: true })on a non-editable visible node → edit opens.event-signalsdemo, Tab through a tree with mixed editable/non-editable nodes; the event stream should be clean.Follow-up (separate PR)
After this lands and is verified stable, remove the now-unused
previouslyEditedElement/recordPreviousEditplumbing fromEditingProviderand audittabDirectionfor the same.🤖 Generated with Claude Code